improved runtime of pre-commit significantly #1534
Conversation
|
Warning Rate limit exceeded@0xDEnYO has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR refactors the Husky pre-commit hook to use bash with conditional, parallel task execution. It adds a Husky shell wrapper, implements staged-file detection, integrates TypeChain and Forge builds, enhances secret scanning with ENV variable parsing, and introduces helper utilities for structured output and filtered logging. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.husky/pre-commit (3)
228-231: Minor: Preferprintfoverechofor null-delimited data.
echomay add a trailing newline that could affect xargs processing. Consider usingprintf '%s'for more predictable behavior with null-delimited strings.- FILES_TO_CHECK=$(git diff --cached --name-only --diff-filter=ACM -z) - FILES_COUNT=$(echo "$FILES_TO_CHECK" | tr '\0' '\n' | wc -l | tr -d ' ') + FILES_TO_CHECK=$(git diff --cached --name-only --diff-filter=ACM -z) + FILES_COUNT=$(printf '%s' "$FILES_TO_CHECK" | tr '\0' '\n' | grep -c . || echo 0)Alternatively, pipe the git command directly:
FILES_COUNT=$(git diff --cached --name-only --diff-filter=ACM | wc -l | tr -d ' ')
252-255: Sameprintfconsideration for xargs input.For consistency with null-delimited data handling:
- echo "$FILES_TO_CHECK" | xargs -0 -n 1 -P "$PARALLEL_JOBS" sh "$TEMP_SCRIPT" > "${SECRET_CHECK_OUTPUT}.results" 2>&1 + printf '%s' "$FILES_TO_CHECK" | xargs -0 -n 1 -P "$PARALLEL_JOBS" sh "$TEMP_SCRIPT" > "${SECRET_CHECK_OUTPUT}.results" 2>&1
374-389: Potential duplicate warning output.The subshell (lines 269-276) already prints warning messages to
SECRET_CHECK_OUTPUT. Then lines 375-386 print additional messages based on the same patterns. For the private key case (exit code 0), this could result in duplicate warnings.Consider removing the redundant message printing in the subshell since the main script handles output display:
- elif echo "$SECRET_RESULT" | grep -q "Potential private key found"; then - printf '\033[36m%s\033[0m\n' "Warning: Potential Ethereum private keys found" - printf '\033[91m%s\033[0m\n' "Check each match carefully before pushing to GitHub" - EXIT_CODE=0 # Warning only - fi + fiOr alternatively, in the main script, skip the secondary warning print since
SECRET_RESULTalready contains the formatted message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.husky/pre-commit(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
.husky/pre-commit
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
.husky/pre-commit
📚 Learning: 2025-08-29T05:16:40.067Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Applied to files:
.husky/pre-commit
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run-unit-tests
- GitHub Check: run-unit-tests
🔇 Additional comments (12)
.husky/pre-commit (12)
4-9: LGTM! Early exit optimization.The early exit check when no files are staged is a good optimization that avoids unnecessary work.
11-18: File type detection looks correct.The single-pass detection using
grep -qEwith short-circuit evaluation is efficient. Note that.jsxfiles are not included—this appears intentional if the project doesn't use JSX without TypeScript.
19-33: LGTM! Proper temp directory setup with cleanup.The trap correctly handles cleanup on EXIT, INT, and TERM signals. The temp file paths are well-organized for parallel execution.
34-52: Good helper functions for readable output.The colored status indicators (
✓,⊘,✗) improve user experience. The print functions are well-structured.
54-80: Output filtering logic is sound.The
|| truepattern correctly prevents grep from returning non-zero when no matches are found. Thehead -20limit on tsc output (line 74) may truncate useful error context, but is reasonable for pre-commit brevity.
82-111: Robust wait-and-check mechanism.The pattern of storing exit codes in files and reading them back is the correct approach for capturing exit status from backgrounded subshells. The fallback to
echo "1"on read failure is a safe default.
150-152: Excellent race condition prevention.Using
git show ":$FILE"to read staged content instead of the working directory file is the correct approach—it prevents race conditions when lint-staged modifies files concurrently.
283-316: Well-structured parallel execution.The pattern of running forge and lint-staged in parallel with proper cleanup on failure is correct. The subshell pattern
(cmd; echo $? > file) &correctly captures exit codes for backgrounded processes.
318-332: LGTM! Correct sequential dependency.TypeChain correctly runs after forge build completes, respecting the dependency that typechain needs compiled artifacts. The summary extraction provides good user feedback.
334-349: Verify: TypeScript-only changes skip type checking.The condition on line 335 requires both TS/JS files AND Solidity files to be changed. This means commits with only TypeScript changes won't run
tsc.If this is intentional for performance (avoiding tsc when typechain types haven't changed), consider adding a comment explaining the rationale. Otherwise, you may want to run tsc whenever
HAS_TS_JS_FILES = yes.-if [ "$HAS_TS_JS_FILES" = "yes" ] && [ "$HAS_SOL_FILES" = "yes" ]; then +# Only run tsc when Solidity files changed (typechain types may have changed) +# TS-only changes rely on lint-staged for basic validation +if [ "$HAS_TS_JS_FILES" = "yes" ] && [ "$HAS_SOL_FILES" = "yes" ]; then
351-358: LGTM! Proper lint-staged completion handling.The wait occurs after other sequential tasks, allowing lint-staged to run in parallel for maximum efficiency while still ensuring it completes before the final status.
391-392: LGTM! Clean completion.The final success message and exit 0 provide clear feedback that all checks passed.
…tracts into improve-pre-commit-runtime
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.husky/_/.gitignore(1 hunks).husky/_/husky.sh(1 hunks).husky/pre-commit(3 hunks)script/deploy/healthCheck.ts(1 hunks)script/playgroundHelpers.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- script/deploy/healthCheck.ts
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.sh
📄 CodeRabbit inference engine (conventions.md)
script/**/*.sh: Bash scripts must begin with #!/bin/bash, use modular functions with clear sections (Logging, Error handling and logging, Deployment functions), follow DRY principle using helper files, use consistent indentation and naming, include proper comments and documentation
Bash scripts must use helper functions for logging (echoDebug, error, warning, success), validate inputs and environment early, check function exit status with checkFailure, use set -e where appropriate, and provide automated retry mechanisms for RPC issues
Files:
script/playgroundHelpers.sh
🧠 Learnings (6)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2025-08-29T05:16:40.067Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Applied to files:
.husky/_/husky.sh.husky/_/.gitignore.husky/pre-commit
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/playgroundHelpers.sh.husky/pre-commit
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/playgroundHelpers.sh.husky/pre-commit
📚 Learning: 2024-11-26T01:18:52.125Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 782
File: script/deploy/deploySingleContract.sh:231-231
Timestamp: 2024-11-26T01:18:52.125Z
Learning: In the `deploySingleContract.sh` script, environment variable assignments preceding the `forge script` command (e.g., `DEPLOYSALT=$DEPLOYSALT ... forge script ...`) are acceptable and function correctly, even when variable expansions like `$DIAMOND_TYPE` and `$NETWORK` are used later in the same command.
Applied to files:
.husky/pre-commit
📚 Learning: 2024-12-05T03:41:16.360Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 894
File: .github/workflows/diamondEmergencyPause.yml:68-68
Timestamp: 2024-12-05T03:41:16.360Z
Learning: Before reporting missing GitHub secrets in the lifinance/contracts repository, conclusively verify that the secret is absent to avoid false alerts.
Applied to files:
.husky/pre-commit
🧬 Code graph analysis (1)
script/playgroundHelpers.sh (1)
script/helperFunctions.sh (1)
error(3117-3119)
🪛 Shellcheck (0.11.0)
.husky/_/husky.sh
[warning] 9-9: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 19-19: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (6)
.husky/_/.gitignore (1)
1-3: Good Husky hygiene: ignore everything except the wrapper.
Keeps onlyhusky.sh(and this.gitignore) tracked, avoiding accidental commits of generated/temporary Husky files.script/playgroundHelpers.sh (1)
720-736: Safer docs: placeholder examples instead of real tx hash / RPC.
This is purely messaging and reduces accidental reuse/leaking of real values..husky/pre-commit (3)
1-14: Shebang change to bash is fine, but please verify Husky executes it as bash everywhere.
This hook now relies on bash features (arrays,[[ ... ]], process substitution). Based on prior learnings about preferring#!/usr/bin/env shin this repo, double-check this doesn’t break anyone’s local hook execution path.
61-295: Secret-check approach looks solid (staged-content scan +.envkey-only reporting).
Scanning staged blobs viagit show ":$FILE"avoids false negatives from unstaged working tree changes, and the output doesn’t echo secret values.Also applies to: 371-399
15-22: The lint-staged and tsc configurations are correct; no coverage regression exists.
- Lint-staged runs on commits with any lintable files (
.solor.(ts|js)), not just when both types change. TheHAS_TYPE_FILESgate is inclusive, not restrictive.tsc-filesis already part of the lint-staged config for*.{ts,js}files, so TS-only commits receive type checking via lint-staged.- JSON/MD/YAML files are not configured in lint-staged, so commits affecting only those file types skipping lint-staged is expected, not a regression.
- The separate tsc invocation (lines 347–362) only runs when Solidity files also changed; this is redundant with lint-staged's tsc-files but not a gap.
.husky/_/husky.sh (1)
1-37: No issues found. The code works correctly across environments, and bothbasename --and string-based exit code comparisons function as intended. Since this file is part of the Husky framework and is already working in the repository's current setup, stylistic portability improvements are not necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
script/deploy/shared/cached-deployment-querier.tsscript/playgroundHelpers.sh
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/099-finish.mdc)
After TypeScript changes, run lint/tests with Bun
Files:
script/deploy/shared/cached-deployment-querier.ts
{src,script,test}/**/*.{sol,ts}
📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)
Validate all external inputs and configuration (including script/env values) explicitly; prefer existing validation helpers (e.g.,
Validatable, config readers) over ad-hoc checks
Files:
script/deploy/shared/cached-deployment-querier.ts
{script,tasks}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/200-typescript.mdc)
{script,tasks}/**/*.ts: TS scripts use.eslintrc.cjsrules,bunx tsx,citty,consola, and env validated via helpers (e.g.,getEnvVar()).
DO NOT use deprecated ethers-based helpers (getProvider,getWalletFromPrivateKeyInDotEnv, etherssendTransaction,ensureBalanceAndAllowanceToDiamond).
Obey.eslintrc.cjs; avoidany; use TypeChain types fromtypechain/directory (e.g.,ILiFi.BridgeDataStruct).
Import organization: Group imports as: external libs (viem, consola, citty, dotenv) → TypeChain types → config files → internal utils/helpers. Usetypeimports for types-only.
Error handling: Use try/catch withconsola.error()for logging; exit withprocess.exit(1)on fatal errors. Provide meaningful error messages.
CLI: usecitty; logging viaconsola; validate env viagetEnvVar(); exit 0/1 appropriately.
New TypeScript helpers must be covered by a colocated*.test.tsfile using Bun (describe/it/expect) with 100% coverage. Cover edge cases and error paths.
Files:
script/deploy/shared/cached-deployment-querier.ts
**/*.sh
📄 CodeRabbit inference engine (.cursor/rules/099-finish.mdc)
After Bash changes, check execution flags/sourcing
**/*.sh: Start bash deployment scripts with#!/bin/bashshebang
Organize bash scripts into functions with clear sections for Logging, Error handling, and Deployment functions
Keep bash code DRY by using helper functions
Load environment variables from.envorconfig.sh; validate required variables early
Use logging helpers (echoDebug,error,warning,success) for consistent logging in bash scripts
ImplementcheckFailurehelper and retries for RPC-sensitive steps in bash scripts
Avoid inline secrets in bash scripts
Use all uppercase variable names in bash scripts (e.g.,NETWORK,CONTRACT_ADDRESS)
Provide usage/help text in bash scripts with clear exit codes
Document TODOs and limits succinctly in bash scripts; maintain consistent indentation and naming
Bash deploy scripts should wrap Foundry with robust retries, modular functions, and helpers
Files:
script/playgroundHelpers.sh
🧠 Learnings (13)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1168
File: script/deploy/_targetState.json:1564-1589
Timestamp: 2025-05-27T12:36:26.987Z
Learning: When reviewing deployment PRs in the lifinance/contracts repository, target state configuration files (like script/deploy/_targetState.json) may be updated for multiple networks even when the PR is focused on deploying to a specific network. The scope should be determined by the PR title and description, not just by all configuration changes present in the files.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/500-github-actions.mdc:0-0
Timestamp: 2025-12-17T10:30:08.158Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Initialize variables before use in shell scripts and add descriptive comments for complex logic blocks
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/300-bash.mdc:0-0
Timestamp: 2025-12-17T10:29:17.159Z
Learning: Applies to **/*.sh : Bash deploy scripts should wrap Foundry with robust retries, modular functions, and helpers
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2024-11-21T08:24:05.881Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 861
File: script/deploy/safe/add-owners-to-safe.ts:8-13
Timestamp: 2024-11-21T08:24:05.881Z
Learning: In `script/deploy/safe/add-owners-to-safe.ts`, validation for network configuration when casting imported JSON data to `NetworksObject` is not required as per the user's preference.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2024-12-04T12:57:56.493Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 875
File: script/utils/fetch-missing-deployments.ts:0-0
Timestamp: 2024-12-04T12:57:56.493Z
Learning: In the TypeScript file `script/utils/fetch-missing-deployments.ts`, the existing regex pattern `/\/\/\/\s*custom:version\s*([\d.]+)/` used for version extraction is sufficient and does not require modification.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2024-12-04T01:59:34.045Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 832
File: deployments/_deployments_log_file.json:23712-23720
Timestamp: 2024-12-04T01:59:34.045Z
Learning: In `deployments/_deployments_log_file.json`, duplicate deployment entries for the same version and address may occur because they correspond to deployments on different networks. These entries are acceptable and should not be flagged as duplicates in future reviews.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/deploy/shared/cached-deployment-querier.tsscript/playgroundHelpers.sh
📚 Learning: 2025-07-03T10:19:21.132Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1140
File: deployments/worldchain.json:28-29
Timestamp: 2025-07-03T10:19:21.132Z
Learning: When reviewing deployment PRs, always perform comprehensive searches for addresses in deployment log files before reporting them as missing. Use multiple search strategies including direct address searches and context-aware searches to avoid false positives about missing entries. Double-check the actual presence of addresses in the deployment logs before flagging inconsistencies.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2025-09-16T01:39:54.099Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.099Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2024-10-04T09:17:19.275Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/avalanche.diamond.json:105-105
Timestamp: 2024-10-04T09:17:19.275Z
Learning: In the `lifinance/contracts` repository, contracts may have different addresses across networks. Do not check if contracts have the same addresses across all networks in future reviews.
Applied to files:
script/deploy/shared/cached-deployment-querier.ts
📚 Learning: 2025-12-17T10:30:26.229Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/600-transaction-analysis.mdc:0-0
Timestamp: 2025-12-17T10:30:26.229Z
Learning: When transaction analysis mode is activated, use `.cursor/commands/analyze-tx.mdc` for complete workflow, rules, and policies
Applied to files:
script/playgroundHelpers.sh
📚 Learning: 2025-12-17T10:30:26.229Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/600-transaction-analysis.mdc:0-0
Timestamp: 2025-12-17T10:30:26.229Z
Learning: Transaction analysis mode is activated ONLY when user provides transaction hash + network name AND asks to analyze/debug that specific transaction, or invokes `/analyze-tx` command
Applied to files:
script/playgroundHelpers.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (1)
script/deploy/shared/cached-deployment-querier.ts (1)
133-138: LGTM!The multi-line reformatting improves readability without changing the underlying logic. The case-insensitive address comparison and null fallback are correct.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| error "Example: analyzeFailingTx arbitrum 0xedc3d7580e0b333f7c232649b0506aa3e811b0f5060d84e75a91b0dec68b4cc9" | ||
| return 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
I did some testing and noticed that for example when I put a blankline between parameters in sol file the old script treats the commit as empty and blocks it. new script also blocks the commit in this case but it doesn’t provide any error message explaining why the commit is rejected. please take a look
miron@MacBook-Pro-Micha contracts % git commit -m "test"
Checking for secrets and private keys...
✓ Secret check passed
━━━ Pre-commit Checks ━━━
Building contracts with forge...
Running lint-staged (formatting & linting)...
✓ Forge build completed
Generating TypeChain types...
✓ Successfully generated 1145 typings!
[STARTED] bash -c 'if [ "$#" -gt 0 ]; then output=$(bunx solhint --noPrompt "$@" 2>&1); ret=$?; if echo "$output" | grep -q "No files to lint!"; then exit 0; else echo "$output"; exit $ret; fi; else exit 0; fi # If no lintable files, exit silently to avoid commit errors' _
[COMPLETED] bash -c 'if [ "$#" -gt 0 ]; then output=$(bunx solhint --noPrompt "$@" 2>&1); ret=$?; if echo "$output" | grep -q "No files to lint!"; then exit 0; else echo "$output"; exit $ret; fi; else exit 0; fi # If no lintable files, exit silently to avoid commit errors' _
[STARTED] Reverting to original state because of errors...
[COMPLETED] Reverting to original state because of errors...
error: "lint-staged" exited with code 1
Lint-staged failed. Aborting commit.
husky - pre-commit hook exited with code 1 (error)
I don't know how it's with different errors but having a reason why it was rejected would be good
Why did I implement it this way?
Sometimes the pre-commit check was taking forever, especially on larger git diffs.
This PR introduces some optimizations and parallelizes tasks more efficiently which significantly reduces the runtime of the checks.
This is vibe-coded, please review carefully.
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)